feat: payload collection instrumentation rule #1488
Conversation
| Language common.ProgrammingLanguage `json:"language"` | ||
| } | ||
|
|
||
| type PayloadCollection struct { |
There was a problem hiding this comment.
What do you think about taking it out to a new directory?
rules/
payload/
payload_collection.go
otherRule/
otherrule.go
There was a problem hiding this comment.
great idea, refactored 👍
| type PayloadCollection struct { | ||
| // Collect HTTP request payload data when available. | ||
| // Can be a client (outgoing) request or a server (incoming) request, depending on the instrumentation library | ||
| HttpRequest *HttpPayloadCollectionRule `json:"httpRequest,omitempty"` |
There was a problem hiding this comment.
I think better to remove the Rule from it's name
these are attributes/options within payloadCollectionRule ?
There was a problem hiding this comment.
make sense. changed
| HeadSamplingConfig HeadSamplingConfig `json:"headSamplerConfig,omitempty"` | ||
| HeadSamplingConfig *HeadSamplingConfig `json:"headSamplerConfig,omitempty"` | ||
|
|
||
| DefaultPayloadCollection PayloadCollection `json:"payloadCollection"` |
There was a problem hiding this comment.
why the defaultPayloadCollection is in the SdkConfig level but the config itself is in InstrumentationLibraryConfig?
There was a problem hiding this comment.
It exists in both, so one can give a default to the SDK (recommended) but then override it with a specific config for a specific instrumentation library
|
|
||
| Instrumentation Rules control how telemetry is recorded from your application. | ||
|
|
||
| ## Rule Types: |
There was a problem hiding this comment.
Maybe worth to mention that soon it will be available from the UI?
There was a problem hiding this comment.
I think the docs should say what we have and not our plans. Also, there isn't anything in the docs at the moment about the ui. I support adding documentation about the UI but not with this PR
| sidebarTitle: "Payload Collection" | ||
| --- | ||
|
|
||
| The "Payload Collection" Rule can be used to add span attributes containing payload data to traces. |
There was a problem hiding this comment.
Adding "star" - enterprise support only
| - `maxPayloadLength` (optional): Maximum length of the payload to collect. If the payload is longer than this value, it will be truncated or dropped, based on the value of `dropPartialPayloads` config option. When not specified (recommended), the instrumentation library will use any reasonable default value | ||
| - `dropPartialPayloads` (optional, default is false): If the payload is larger than the MaxPayloadLength, this parameter will determine if the payload should be partially collected up to the allowed length, or not collected at all. This is useful if you require some decoding of the payload (like json) and having it partially is not useful. | ||
|
|
||
| - `dbQuery` (optional): Collect database query payload info when available. |
There was a problem hiding this comment.
I think need to mention Added attributes
There was a problem hiding this comment.
I will add it once the implementation in the eBPF is changed to use it
| @@ -98,3 +98,11 @@ rules: | |||
| - patch | |||
| - update | |||
| - watch | |||
| - apiGroups: | |||
There was a problem hiding this comment.
Just comment: Need to add also in the legacy chart one right?
There was a problem hiding this comment.
| } | ||
|
|
||
| // delete all existing sdk configs to re-calculate them | ||
| ic.Spec.SdkConfigs = []odigosv1alpha1.SdkConfig{} |
There was a problem hiding this comment.
Maybe better create a new []odigosv1alpha1.SdkConfig{} object in memory and replace only at the end of the function?
There was a problem hiding this comment.
like it. refactored
| Scheme *runtime.Scheme | ||
| } | ||
|
|
||
| func (r *InstrumentedApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { |
There was a problem hiding this comment.
Why we also need to updateInstrumentationConfigForWorkload in the instrumentedapplication reconcile ?
There was a problem hiding this comment.
Once the language is detected, we need to update the config for this workload. Otherwise it will remain empty until a new rule is added
No description provided.